repo: Further optimize `ostree_repo_list_objects_set()`

In a prior change we discovered that for bad historical reasons
libostree was returning a mapping "object type+checksum" => "metadata"
but the "metadata" was redundant and pointless.

Optimize the prune API to use a (currently internal) object listing
API which returns a set, not a map.  This allows `GHashTable` to
avoid allocating a separate array for the values, neatly cutting
memory usage in half (from ~13MB to ~6MB) on my test case of a
dry-run prune of a FCOS build.

diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h
index 96253e777816162a67a0eed65fec542acfdd4348..3858a36e0da2ae3b4ecb55b47d5199ecae540c0d 100644 (file)
--- a/src/libostree/ostree-repo-private.h
+++ b/src/libostree/ostree-repo-private.h
@@ -513,6 +513,12 @@ _ostree_repo_verify_bindings (const char  *collection_id,
                               GVariant    *commit,
                               GError     **error);
 
+GHashTable *
+ostree_repo_list_objects_set (OstreeRepo                  *self,
+                              OstreeRepoListObjectsFlags   flags,
+                              GCancellable                *cancellable,
+                              GError                     **error);
+
 /**
  * OstreeRepoAutoTransaction:
  *

diff --git a/src/libostree/ostree-repo-prune.c b/src/libostree/ostree-repo-prune.c
index 0c702dc9d61e3a957eca985b2b25a723e1cd0fb4..ccd557824e97e6dd856f1d88d2b6963fcf3b325b 100644 (file)
--- a/src/libostree/ostree-repo-prune.c
+++ b/src/libostree/ostree-repo-prune.c
@@ -280,15 +280,8 @@ repo_prune_internal (OstreeRepo        *self,
   g_autoptr(GHashTable) reachable_owned = g_hash_table_ref (options->reachable);
   data.reachable = reachable_owned;
 
-  GLNX_HASH_TABLE_FOREACH_KV (objects, GVariant*, serialized_key, GVariant*, objdata)
+  GLNX_HASH_TABLE_FOREACH (objects, GVariant*, serialized_key)
     {
-      gboolean is_loose;
-
-      g_variant_get_child (objdata, 0, "b", &is_loose);
-
-      if (!is_loose)
-        continue;
-
       if (!maybe_prune_loose_object (&data, options->flags, serialized_key,
                                      cancellable, error))
         return FALSE;
@@ -444,8 +437,9 @@ ostree_repo_prune (OstreeRepo        *self,
         return FALSE;
     }
 
-  if (!ostree_repo_list_objects (self, OSTREE_REPO_LIST_OBJECTS_ALL | OSTREE_REPO_LIST_OBJECTS_NO_PARENTS,
-                                 &objects, cancellable, error))
+  objects = ostree_repo_list_objects_set (self, OSTREE_REPO_LIST_OBJECTS_ALL | OSTREE_REPO_LIST_OBJECTS_NO_PARENTS,
+                                          cancellable, error);
+  if (!objects)
     return FALSE;
 
   if (!refs_only)

diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index 5b1aaae9bef4a5f020f4377e3b08c3464aff3cf9..e823e19e2ec040ad7db381bac5b5fa971d6e2855 100644 (file)
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -3974,8 +3974,11 @@ list_loose_objects_at (OstreeRepo             *self,
       key = ostree_object_name_serialize (buf, objtype);
 
       /* transfer ownership */
-      g_hash_table_replace (inout_objects, g_variant_ref_sink (key),
-                            g_variant_ref (dummy_value));
+      if (dummy_value)
+        g_hash_table_replace (inout_objects, g_variant_ref_sink (key),
+                              g_variant_ref (dummy_value));
+      else
+        g_hash_table_add (inout_objects, g_variant_ref_sink (key));
     }
 
   return TRUE;
@@ -3983,15 +3986,13 @@ list_loose_objects_at (OstreeRepo             *self,
 
 static gboolean
 list_loose_objects (OstreeRepo                     *self,
+                    GVariant                       *dummy_value,
                     GHashTable                     *inout_objects,
                     const char                     *commit_starting_with,
                     GCancellable                   *cancellable,
                     GError                        **error)
 {
   static const gchar hexchars[] = "0123456789abcdef";
-  // For unfortunate historical reasons we emit this dummy value.
-  g_autoptr(GVariant) dummy_loose_object_variant =
-    g_variant_ref_sink (g_variant_new ("(b@as)", TRUE, g_variant_new_strv (NULL, 0)));
 
   for (guint c = 0; c < 256; c++)
     {
@@ -3999,7 +4000,7 @@ list_loose_objects (OstreeRepo                     *self,
       buf[0] = hexchars[c >> 4];
       buf[1] = hexchars[c & 0xF];
       buf[2] = '\0';
-      if (!list_loose_objects_at (self, dummy_loose_object_variant,
+      if (!list_loose_objects_at (self, dummy_value,
                                   inout_objects, self->objects_dir_fd, buf,
                                   commit_starting_with,
                                   cancellable, error))
@@ -4905,47 +4906,31 @@ ostree_repo_load_commit (OstreeRepo            *self,
                                  out_variant, NULL, NULL, out_state, NULL, error);
 }
 
-/**
- * ostree_repo_list_objects:
- * @self: Repo
- * @flags: Flags controlling enumeration
- * @out_objects: (out) (transfer container) (element-type GVariant GVariant):
- * Map of serialized object name to variant data
- * @cancellable: Cancellable
- * @error: Error
- *
- * This function synchronously enumerates all objects in the
- * repository, returning data in @out_objects.  @out_objects
- * maps from keys returned by ostree_object_name_serialize()
- * to #GVariant values of type %OSTREE_REPO_LIST_OBJECTS_VARIANT_TYPE.
- *
- * Returns: %TRUE on success, %FALSE on error, and @error will be set
- */
-gboolean
-ostree_repo_list_objects (OstreeRepo                  *self,
-                          OstreeRepoListObjectsFlags   flags,
-                          GHashTable                 **out_objects,
-                          GCancellable                *cancellable,
-                          GError                     **error)
+static GHashTable *
+repo_list_objects_impl (OstreeRepo                  *self,
+                        OstreeRepoListObjectsFlags   flags,
+                        GVariant                    *dummy_value,
+                        GCancellable                *cancellable,
+                        GError                     **error)
 {
-  g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
-  g_return_val_if_fail (self->inited, FALSE);
+  g_assert (error == NULL || *error == NULL);
+  g_assert (self->inited);
 
   g_autoptr(GHashTable) ret_objects =
     g_hash_table_new_full (ostree_hash_object_name, g_variant_equal,
                            (GDestroyNotify) g_variant_unref,
-                           (GDestroyNotify) g_variant_unref);
+                           dummy_value ? (GDestroyNotify) g_variant_unref : NULL);
 
   if (flags & OSTREE_REPO_LIST_OBJECTS_ALL)
     flags |= (OSTREE_REPO_LIST_OBJECTS_LOOSE | OSTREE_REPO_LIST_OBJECTS_PACKED);
 
   if (flags & OSTREE_REPO_LIST_OBJECTS_LOOSE)
     {
-      if (!list_loose_objects (self, ret_objects, NULL, cancellable, error))
+      if (!list_loose_objects (self, dummy_value, ret_objects, NULL, cancellable, error))
         return FALSE;
       if ((flags & OSTREE_REPO_LIST_OBJECTS_NO_PARENTS) == 0 && self->parent_repo)
         {
-          if (!list_loose_objects (self->parent_repo, ret_objects, NULL, cancellable, error))
+          if (!list_loose_objects (self->parent_repo, dummy_value, ret_objects, NULL, cancellable, error))
             return FALSE;
         }
     }
@@ -4955,7 +4940,59 @@ ostree_repo_list_objects (OstreeRepo                  *self,
       /* Nothing for now... */
     }
 
-  ot_transfer_out_value (out_objects, &ret_objects);
+  return g_steal_pointer (&ret_objects);
+}
+
+/* A currently-internal version of ostree_repo_list_objects which returns
+ * a set, and not a map (with a useless value).
+ */
+GHashTable *
+ostree_repo_list_objects_set (OstreeRepo                  *self,
+                              OstreeRepoListObjectsFlags   flags,
+                              GCancellable                *cancellable,
+                              GError                     **error)
+{
+  return repo_list_objects_impl (self, flags, NULL, cancellable, error);
+}
+
+/* For unfortunate historical reasons we emit this dummy value.
+ * It was intended to provide additional information about the object (e.g. "is in a pack file")
+ * but we ended up not shipping pack files.
+ */
+static GVariant *
+get_dummy_list_objects_variant (void)
+{
+  return g_variant_ref_sink (g_variant_new ("(b@as)", TRUE, g_variant_new_strv (NULL, 0)));
+}
+
+/**
+ * ostree_repo_list_objects:
+ * @self: Repo
+ * @flags: Flags controlling enumeration
+ * @out_objects: (out) (transfer container) (element-type GVariant GVariant):
+ * Map of serialized object name to variant data
+ * @cancellable: Cancellable
+ * @error: Error
+ *
+ * This function synchronously enumerates all objects in the
+ * repository, returning data in @out_objects.  @out_objects
+ * maps from keys returned by ostree_object_name_serialize()
+ * to #GVariant values of type %OSTREE_REPO_LIST_OBJECTS_VARIANT_TYPE.
+ *
+ * Returns: %TRUE on success, %FALSE on error, and @error will be set
+ */
+gboolean
+ostree_repo_list_objects (OstreeRepo                  *self,
+                          OstreeRepoListObjectsFlags   flags,
+                          GHashTable                 **out_objects,
+                          GCancellable                *cancellable,
+                          GError                     **error)
+{
+  g_autoptr(GVariant) dummy_value = get_dummy_list_objects_variant ();
+  g_autoptr(GHashTable) ret = repo_list_objects_impl (self, flags, dummy_value, cancellable, error);
+  if (!ret)
+    return FALSE;
+  ot_transfer_out_value (out_objects, &ret);
   return TRUE;
 }
 
@@ -4987,13 +5024,14 @@ ostree_repo_list_commit_objects_starting_with (OstreeRepo                  *self
     g_hash_table_new_full (ostree_hash_object_name, g_variant_equal,
                            (GDestroyNotify) g_variant_unref,
                            (GDestroyNotify) g_variant_unref);
+  g_autoptr(GVariant) dummy_loose_object_variant = get_dummy_list_objects_variant ();
 
-  if (!list_loose_objects (self, ret_commits, start, cancellable, error))
+  if (!list_loose_objects (self, dummy_loose_object_variant, ret_commits, start, cancellable, error))
     return FALSE;
 
   if (self->parent_repo)
     {
-      if (!list_loose_objects (self->parent_repo, ret_commits, start,
+      if (!list_loose_objects (self->parent_repo, dummy_loose_object_variant, ret_commits, start,
                                cancellable, error))
         return FALSE;
     }